Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New lint: iter_next_slice #5597

Merged
merged 1 commit into from
Jun 2, 2020
Merged

Conversation

esamudera
Copy link
Contributor

@esamudera esamudera commented May 14, 2020

Hello, this is a work-in-progress PR for issue: #5572

I have implemented lint to replace iter().next() for slice[index..] and array with get(index) and get(0) respectively. However since I made a lot of changes, I would like to request some feedback before continuing so that I could fix mistakes.

Thank you!


changelog: implement iter_next_slice lint and test, and modify needless_continues, for_loop_over_options_result UI tests since they have iter().next()

@bors
Copy link
Collaborator

bors commented May 16, 2020

☔ The latest upstream changes (presumably #5563) made this pull request unmergeable. Please resolve the merge conflicts.

tests/ui/for_loop_over_option_result.rs Outdated Show resolved Hide resolved
clippy_lints/src/loops.rs Outdated Show resolved Hide resolved
@flip1995 flip1995 added the S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) label May 21, 2020
@flip1995 flip1995 self-assigned this May 25, 2020
clippy_lints/src/methods/mod.rs Show resolved Hide resolved
tests/ui/iter_next_slice.rs Outdated Show resolved Hide resolved
@@ -0,0 +1,23 @@
#![warn(clippy::iter_next_slice)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#![warn(clippy::iter_next_slice)]
// run-rustfix
#![warn(clippy::iter_next_slice)]

and a update-all-references.sh run and this PR should be finished.

Copy link
Contributor Author

@esamudera esamudera May 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I have added the run-rustfix marker and iter_next_slice.fixed in the new commit.

However, the test failed at Clippy Test, cargo build stage. I'm not sure about the errors listed, is it about toolchain version incompatibility? Should I rebase this branch to the latest master?

Copy link
Member

@flip1995 flip1995 May 31, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may have to rebase after #5665 is merged. In the meantime you could go ahead, remove the "WIP" prefixes from your commits and remove the "draft" status from this PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I have squashed all the commits into one, and have updated this PR status to Open.

@esamudera esamudera changed the title WIP - implement iter next slice New lint: iter_next_slice May 31, 2020
@esamudera esamudera marked this pull request as ready for review May 31, 2020 20:12
@esamudera
Copy link
Contributor Author

esamudera commented May 31, 2020

The test results are in and perfect! Looks like this PR is done. Thank you @flip1995 for your instructions and for accommodating newcomers like me, I will use this PR as a reference to work on other issues 👍

@flip1995
Copy link
Member

Thanks for your contribution! Always happy to get new contributors on their way.

@bors r+

@bors
Copy link
Collaborator

bors commented May 31, 2020

📌 Commit 32fde0b has been approved by flip1995

bors added a commit that referenced this pull request May 31, 2020
New lint: iter_next_slice

Hello, this is a work-in-progress PR for issue: #5572

I have implemented lint to replace `iter().next()` for `slice[index..]` and `array` with `get(index)` and `get(0)` respectively. However since I made a lot of changes, I would like to request some feedback before continuing so that I could fix mistakes.

Thank you!

---

changelog:
- implement iter next slice lint and test
- modify needless_continues, for_loop_over_options_result UI tests since they have `iter().next()`
@bors
Copy link
Collaborator

bors commented May 31, 2020

⌛ Testing commit 32fde0b with merge 9d62433...

@bors
Copy link
Collaborator

bors commented May 31, 2020

💔 Test failed - checks-action_test

@flip1995
Copy link
Member

flip1995 commented Jun 2, 2020

@bors retry

@bors
Copy link
Collaborator

bors commented Jun 2, 2020

⌛ Testing commit 32fde0b with merge f760d77...

@bors
Copy link
Collaborator

bors commented Jun 2, 2020

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: flip1995
Pushing f760d77 to master...

@bors bors merged commit f760d77 into rust-lang:master Jun 2, 2020
@ThibsG
Copy link
Contributor

ThibsG commented Jun 2, 2020

I think the issue linked to this PR can be closed manually.

On a side note @esamudera , you can setup a PR to automatically close an issue when merged by adding a line in your PR description (like you did for changelog):

fixes: #5572

It works also with closes or resolves keywords 😉

@esamudera
Copy link
Contributor Author

Okay, thank you for the tip, I will add the issue keyword on my next PR 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants